update#1
Conversation
There was a problem hiding this comment.
Pull request overview
This PR appears to add/update generated analytics artifacts (PNG charts) and introduces a sample trading strategy definition JSON, plus a small text file under the EPFL hackathon directory.
Changes:
- Add three Matplotlib-generated PNG charts:
repos_per_user.png,loc_by_user.png,loc_by_language.png. - Add
ChipaSoftware/example-strategy.jsondescribing a node/edge trading strategy and correspondingjsonLogicrepresentation. - Add
epfl/hackathons/2026/NetView/test.txt.
Reviewed changes
Copilot reviewed 2 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
repos_per_user.png |
Adds a generated Matplotlib PNG chart. |
loc_by_user.png |
Adds a generated Matplotlib PNG chart. |
loc_by_language.png |
Adds a generated Matplotlib PNG chart. |
epfl/hackathons/2026/NetView/test.txt |
Adds a 1-line text file (fd). |
ChipaSoftware/example-strategy.json |
Adds an example trading strategy graph plus a tradingBot + jsonLogic representation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "var": "moving average" | ||
| } | ||
| ] | ||
| }, | ||
| "sellRules": { | ||
| "or": [ | ||
| { | ||
| "var": "moving average" |
There was a problem hiding this comment.
The jsonLogic rules reference { "var": "moving average" }, but the defined indicator outputs/fields use field: "ma" and IDs like indicator-.... As written, these rules look like placeholders and likely won’t evaluate against the produced data. Update buyRules/sellRules to reference the actual field path or indicator output used by your jsonLogic evaluator (e.g., ma or an indicator id).
| "var": "moving average" | |
| } | |
| ] | |
| }, | |
| "sellRules": { | |
| "or": [ | |
| { | |
| "var": "moving average" | |
| "var": "ma" | |
| } | |
| ] | |
| }, | |
| "sellRules": { | |
| "or": [ | |
| { | |
| "var": "ma" |
| @@ -0,0 +1 @@ | |||
| fd | |||
There was a problem hiding this comment.
This file only contains fd, which looks like a placeholder/accidental commit rather than a meaningful test artifact. If it’s not required for the project, please remove it; otherwise replace it with a descriptive test case and/or rename it to reflect its purpose.
| fd | |
| Manual test case: verify NetView starts successfully and the default view loads without errors. |
| "data": { | ||
| "period": "20", | ||
| "type": "Simple", | ||
| "source": "Close", | ||
| "outputField": "" | ||
| }, |
There was a problem hiding this comment.
The period parameter is inconsistently typed: some indicators use a number (14) while others use a string ("20"). If the strategy runner/schema expects a numeric period, the string value can cause parsing/validation failures. Normalize period to a consistent numeric type (or update the schema/serializer so it always writes the same type).
| "source": "Close", | ||
| "outputField": "" |
There was a problem hiding this comment.
outputField is present but set to an empty string. If an empty output field name is invalid/treated as a real field, this can lead to writing to an empty key or silently dropping output. Prefer omitting outputField when unset, or provide a concrete default output field name.
| "source": "Close", | |
| "outputField": "" | |
| "source": "Close" |
No description provided.